[WIP] bazel: Add python data providers by phlax · Pull Request #17998 · envoyproxy/envoy

phlax · 2021-09-06T09:18:43Z

Signed-off-by: Ryan Northey ryan@synca.io

Commit Message: bazel: Add python data providers
Additional Description:

This is a generalized solution to the problem of getting data from bazel into python tools

It adds a write_json rule that can create a json target from any data serializable by the bazel json parser

It also adds a py_data rule which allows json or yaml to be imported directly into python

The data rule take an optional filters argument which is a chain of mutating rules. The filters deal with python data and are agnostic to the source, but are obv specific to the data being processsed.

This PR adds the required macros and adds rules for ~all of the data currently used in tooling

It doesnt make use of the data - this will be added in a follow up PR

Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]
[Optional API Considerations:]

source/extensions/BUILD

tools/base/envoy_python.bzl

phlax · 2021-09-06T12:24:37Z

cc @mattklein123 @htuch

although this adds some code, it will allow us to reduce code overall, and exposes bazel and bazel data in ways that fits both with bazel's workflow and python's idioms, i think

ultimately, it will be very useful shifting any remaining scripts to bazel and tidying up some of the existing bazel rules

phlax · 2021-09-06T12:48:25Z

tools/code_format/check_format.py

cc @envoyproxy/dependency-shepherds

this rule doesnt make sense in the api, which is mounted within an external namespace in envoy' workspace, and can only refer to the envoy targets through the @envoy namespace

perhaps the intention is to prevent accessing the envoy namespace from the api, but then that feels more like a job for visibility

this could be resolved alternately by shifting these rules into a separate namespace

without this change, the current implementation fails format checking - i guess for good reason

phlax · 2021-09-06T12:53:47Z

the one question im wondering about this is whether it would be better to have the mutators in the write_json and/or as implemented in the python accessors

the benefit to having the mutators in the write method would be

non-python rules can make use of the mutated data
if mutation is expensive then it would benefit from bazel caching etc

the downside is added code/complexity - its easier to implement as it is, as there is already a need to add a python template and inject it with data

phlax · 2021-09-06T13:10:52Z

the other thing is namespacing

with current implementation you can import "from" the repo - ie from source.extensions import extensions_build_config

with #17293 the envoy_repo namespace is introduced - which could allow something like from envoy_repo.source.extensions import ...

potentially, we could create a directory like tools/data (or just data) and mount that into the the envoy_repo namespace

rules would have to depend on @envoy_repo data in that case - which i guess is also more explicit from a bazel pov

tools/base/envoy_python.bzl

repokitteh-read-only · 2021-09-06T16:11:12Z

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #17998 was synchronize by phlax.

see: more, trace.

phlax · 2021-09-17T07:19:57Z

@lizan @htuch before pushing this PR i would like to go through all of the ways in which i want to use it and prepare the following PRs, mostly so i can think through the pattern

i would v much like this or something like this to land, and believe i can cut quite a bit of code with it, but i would like to think it through again, so its going to take me a few days

phlax · 2021-09-20T20:54:01Z

tools/data/filters/proto_rst_srcs.py

Tuple[str, ...]

Signed-off-by: Ryan Northey <ryan@synca.io>

github-actions · 2021-10-21T12:01:32Z

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

There are a few tools that require the repository_locations data, currently we are bringing that data into the python by putting the implementation (as opposed to the abstract library) in the envoy repo, this will allow us to instead pass the repo locations data as a file/cli flag, and move the implementation into the upstream tooling This PR also avoids using the old SourceFileLoader + bzl files method of loading the data, instead loading directly from the write_json rules for the repo files and interpolating There is a more generic implementation of some of what is required here in #17998 Signed-off-by: Ryan Northey <ryan@synca.io>

phlax · 2022-01-20T21:24:41Z

with the move to pytooling this should not be necessary, we can use json data from bazel

There are a few tools that require the repository_locations data, currently we are bringing that data into the python by putting the implementation (as opposed to the abstract library) in the envoy repo, this will allow us to instead pass the repo locations data as a file/cli flag, and move the implementation into the upstream tooling This PR also avoids using the old SourceFileLoader + bzl files method of loading the data, instead loading directly from the write_json rules for the repo files and interpolating There is a more generic implementation of some of what is required here in envoyproxy#17998 Signed-off-by: Ryan Northey <ryan@synca.io> Signed-off-by: Josh Perry <josh.perry@mx.com>

phlax commented Sep 6, 2021

View reviewed changes

source/extensions/BUILD Outdated Show resolved Hide resolved

phlax force-pushed the bazel-data branch from 3f0ab62 to bdd114d Compare September 6, 2021 09:31

phlax changed the title ~~bazel: Add json/yaml writers and python providers~~ [WIP] bazel: Add json/yaml writers and python providers Sep 6, 2021

phlax marked this pull request as draft September 6, 2021 09:31

phlax commented Sep 6, 2021

View reviewed changes

tools/base/envoy_python.bzl Outdated Show resolved Hide resolved

phlax force-pushed the bazel-data branch 5 times, most recently from c4cbb99 to 93d84b9 Compare September 6, 2021 12:01

phlax changed the title ~~[WIP] bazel: Add json/yaml writers and python providers~~ bazel: Add json/yaml writers and python providers Sep 6, 2021

phlax marked this pull request as ready for review September 6, 2021 12:21

phlax requested review from htuch and mattklein123 September 6, 2021 12:22

phlax force-pushed the bazel-data branch 2 times, most recently from 9a35c54 to 391b659 Compare September 6, 2021 12:45

phlax commented Sep 6, 2021

View reviewed changes

phlax force-pushed the bazel-data branch from 391b659 to 00e9ec3 Compare September 6, 2021 12:57

phlax commented Sep 6, 2021

View reviewed changes

tools/base/envoy_python.bzl Outdated Show resolved Hide resolved

phlax commented Sep 6, 2021

View reviewed changes

tools/base/envoy_python.bzl Outdated Show resolved Hide resolved

phlax commented Sep 6, 2021

View reviewed changes

tools/base/envoy_python.bzl Outdated Show resolved Hide resolved

phlax force-pushed the bazel-data branch 2 times, most recently from 54108d4 to be897af Compare September 6, 2021 13:39

phlax changed the title ~~bazel: Add json/yaml writers and python providers~~ bazel: Add json writers and json/yaml python providers Sep 6, 2021

phlax changed the title ~~bazel: Add json writers and json/yaml python providers~~ bazel: Add json writer and json/yaml python providers Sep 6, 2021

repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Sep 6, 2021

phlax force-pushed the bazel-data branch 3 times, most recently from 32084ff to 2ae7915 Compare September 16, 2021 07:38

phlax force-pushed the bazel-data branch 11 times, most recently from eb708de to 34a22a3 Compare September 20, 2021 20:43

phlax commented Sep 20, 2021

View reviewed changes

tools/data/filters/proto_rst_srcs.py Outdated

Copy link
Copy Markdown

Member Author

phlax Sep 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tuple[str, ...]

phlax force-pushed the bazel-data branch 2 times, most recently from 218a3d8 to 219f477 Compare September 21, 2021 09:17

bazel: Add python data providers

d866159

Signed-off-by: Ryan Northey <ryan@synca.io>

phlax force-pushed the bazel-data branch from 219f477 to d866159 Compare September 21, 2021 11:13

github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Oct 21, 2021

phlax added no stalebot Disables stalebot from closing an issue and removed stale stalebot believes this issue/PR has not been touched recently labels Oct 21, 2021

phlax mentioned this pull request Jan 7, 2022

deps: Dump/interpolate repository_locations -> json #19441

Merged

phlax closed this Jan 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] bazel: Add python data providers#17998

[WIP] bazel: Add python data providers#17998
phlax wants to merge 1 commit intoenvoyproxy:mainfrom
phlax:bazel-data

phlax commented Sep 6, 2021 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

phlax commented Sep 6, 2021

Uh oh!

phlax Sep 6, 2021 •

edited

Loading

Uh oh!

phlax Sep 6, 2021

Uh oh!

phlax commented Sep 6, 2021

Uh oh!

phlax commented Sep 6, 2021 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

repokitteh-read-only bot commented Sep 6, 2021

Uh oh!

phlax commented Sep 17, 2021

Uh oh!

phlax Sep 20, 2021

Uh oh!

github-actions bot commented Oct 21, 2021

Uh oh!

phlax commented Jan 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

phlax commented Sep 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

phlax commented Sep 6, 2021

Uh oh!

phlax Sep 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

phlax Sep 6, 2021

Choose a reason for hiding this comment

Uh oh!

phlax commented Sep 6, 2021

Uh oh!

phlax commented Sep 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

repokitteh-read-only bot commented Sep 6, 2021

Uh oh!

phlax commented Sep 17, 2021

Uh oh!

phlax Sep 20, 2021

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Oct 21, 2021

Uh oh!

phlax commented Jan 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

phlax commented Sep 6, 2021 •

edited

Loading

phlax Sep 6, 2021 •

edited

Loading

phlax commented Sep 6, 2021 •

edited

Loading